-
Notifications
You must be signed in to change notification settings - Fork 10.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Overhaul swift_dynamicCast #29658
Overhaul swift_dynamicCast #29658
Conversation
WIP: I still have a couple of failing tests on Linux that I need to resolve. |
@swift-ci Please benchmark |
@swift-ci test windows platform |
Performance: -O
Code size: -OPerformance: -Osize
Code size: -OsizePerformance: -Onone
Code size: -swiftlibsHow to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview
|
Hmmm.... Guess I should take a careful look at casting from Obj-C NSDictionary/NSSet before merging this. |
It might also be worth coordinating with @Catfish-Man to see if bridging conversions can be put on a faster path in general. In particular, if they could avoid having to do dynamic conformance lookups, that'd probably be a big win. |
@swift-ci Please benchmark |
Performance: -O
Code size: -OPerformance: -Osize
Code size: -OsizePerformance: -Onone
Code size: -swiftlibsHow to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview
|
I seem to have fixed the bad perf regression from the initial version. I tinkered some but didn't find any easy fixes for the remaining more minor regressions. I believe I have fixed the last test failure on macOS, so should be ready for CI there. |
@swift-ci Please test macOS |
Build failed |
@swift-ci Please test Linux |
Build failed |
Some of the new tests are failing on Linux when the tests are run in optimized builds. This is expected; this work does not address failures in the cast optimizer. I'll disable those tests in optimized builds for now. |
@swift-ci Please test Linux |
Build failed |
@swift-ci please test Windows platform |
1 similar comment
@swift-ci please test Windows platform |
@swift-ci Please smoke test macOS |
@swift-ci Please smoke test Linux |
1 similar comment
@swift-ci Please smoke test Linux |
@swift-ci Please test macOS |
Build failed |
@swift-ci Please test macOS |
Build failed |
fe92c46
to
42b6fc0
Compare
@swift-ci Please test macOS |
Build failed |
Building out a separate PR #33473 with a more ambitious set of tests. That PR includes the BoxingCasts test suite from this PR and a new suite based on the Casting spec. That PR runs all the tests with both -O and -Onone as well. I plan to break out pieces of this PR into separate PRs and merge them while using PR #33473 to track overall progress towards full compliance with the expected behavior. |
42b6fc0
to
08f65aa
Compare
c3da019
to
70fdc8e
Compare
This includes the rewritten runtime, several compiler fixes, and many test changes. This is (crudely) rebased on top of master to try to minimize confusion. TODO: It would be nice to break this out into several commits.
This validation test exercises a large matrix of types and invariants for dynamic casting. It's formulated as a Python script that emits a number of Swift test programs, compiles, and executes them. The programs are compiled in Swift 4 and 5 mode, with -O and -Onone. The invariants used by these tests follow the specification presented in PR swiftlang#33010. It should be easy to add more as desired. I've tried to design this in such a way that CI logs can provide enough information to narrowly identify the problem area: Separate test cases are generated for each invariant and each type (in particular, this helps with compiler crashes that report the full body of the function in question). The files and test suites are named to identify the optimization mode. The goal of this test suite is to cover a broad cross-section of casting capabilities and combinations, and to make it easy to expand the matrix of combinations. New invariants can easily be added and applied to many types; as new types are added to this test, they can exploit the existing invariants and be exercised across all optimization modes.
Generally, if you can cast something _to_ a box type (Any, AnyObject, existential, _SwiftValue, AnyHashable, etc), then you should be able to cast back to the original type. This test suite simply exercises a large number of such paths. This has been the source of many bugs with the previous implementation.
70fdc8e
to
7bbef79
Compare
This work was submitted via #33561 and related PRs, so this PR is no longer relevant. |
Background
swift_dynamicCast()
is the runtime support behind theas?
,as!
, andis
operators. It attempts to convert a source value with a given type into a
destination location with another specified type.
The previous code had evolved over several years into a tangle of different
strategies for different combinations of source and destination types, with a
growing set of inconsistencies between them. The new implementation has a
unified structure that handles common issues with common code to help ensure
consistent behavior.
Design
The new implementation has a main driver called
tryCast()
that recursivelyattempts to find a suitable conversion from source to destination. For
example, if the source is an
Any
container,tryCast
will extract andrecursively try to convert the contents of that container. At each step,
the
tryCast
will call a functiontryCastToXyz
that understands specificrequirements for a single target type.
Suppose, for example, you have an
Any
containing anOptional<Int>
andyou want to cast that to a
FixedWidthInteger
existential:tryCast()
will asktryCastToOpaqueExistential
if it can convert theAny
directly. This will fail because
Any
does not directly conform to anyprotocols.
tryCast()
will extract theOptional<Int>
from theAny
and try again.tryCast()
will then try to unwrap the optional. If the optional is nil,tryCast
will try to initialize the existential with nil and fail.Finally,
tryCast()
will asktryCastToOpaqueExistential
to convert anInt
to the desired
FixedWidthInteger
existential. This will succeed and theresult is returned.
In addition to unwrapping boxed values,
tryCast()
is also responsible forother general conversion strategies: using the dynamic type instead of
the static type; invoking Objective-C bridging machinery; packing values
into a
__SwiftValue
container.Perhaps most importantly, this PR adds a lot of new tests, including some
"expected failure" tests for issues that aren't (yet) actually fixed.
Resolves SR-1999
Resolves SR-2289 (rdar://27116100)
Resolves SR-4552 (rdar://59174750)
Resolves SR-6126
Resolves SR-8964 (rdar://45217461)
Resolves rdar://58650899
This implementation also does a better job with existential metatypes
and CoreFoundation types.
Next Steps
This PR only changes the runtime dynamic cast code. The Swift compiler
aggressively optimizes casts in release builds, so these changes will
not impact many problems with casting in release builds. I hope to
work on that very soon. In the meantime, you can work around many
compiler problems by calling the following function, which forces
the cast to be handled by the runtime code:
I've not specifically looked at performance. Hopefully, the
new design will make it easier to speed up cast performance in
a future PR.
Caveats
Many casts now succeed that used to fail. Code that relied on certain
casts failing may be surprised. I found and changed one such place
in the standard library.
Different error messages. When a force-cast (
as!
) fails, the messagemay be different, due to the different way in which the new code
tracks and reports failures. I've also taken this opportunity to detect
and issue a specialized error if the cast logic detects an unexpected
null pointer (this can happen when ill-behaved C or Obj-C code returns an
unexpected null pointer).